Skip to content

Fix filtered CAGRA search returning incorrect recall due to kHostMmap#2303

Open
rupakroynv wants to merge 2 commits into
NVIDIA:mainfrom
rupakroynv:fix/cagra-filtered-search-kdevice
Open

Fix filtered CAGRA search returning incorrect recall due to kHostMmap#2303
rupakroynv wants to merge 2 commits into
NVIDIA:mainfrom
rupakroynv:fix/cagra-filtered-search-kdevice

Conversation

@rupakroynv

Copy link
Copy Markdown

Description

Filtered CAGRA search returns near-zero recall when the benchmark harness passes the filter bitset using MemoryType::kHostMmap. This affects the ANN benchmark (CUVS_CAGRA_ANN_BENCH) when running filtered search workloads.

Root Cause

cuvs_cagra::get_preference() returned MemoryType::kHostMmap for both the dataset and the filter bitset. kHostMmap allocates a file-backed MAP_PRIVATE mmap where pages are not pre-faulted — the CPU page table has no mappings until a page fault occurs on first access.

Inside CAGRA's search dispatch (cagra.cuh), bitset_view::count() is called to estimate the filtering rate. That function wraps the host mmap pointer in raft::make_device_vector_view and passes it to raft::popc, which launches a CUDA kernel treating the host pointer as device memory. On non-ATS GPUs this silently reads invalid memory, producing wrong counts and corrupting graph traversal. On ATS-capable GPUs the kernel can access host memory via hardware page migration, but the un-faulted file-backed pages trigger a cascade of serialized ATS page faults across thousands of concurrent graph-traversal threads, causing severe performance degradation.

Fix

Three changes in cpp/bench/ann/src/cuvs/cuvs_cagra_wrapper.h:

  1. get_preference(): Change dataset_memory_type from kHostMmap to kDevice. This causes the benchmark harness to place both the dataset and the filter bitset in device memory before passing them to CAGRA, so count() and all GPU accesses operate on valid device pointers.

  2. set_search_param(): Skip the redundant copy_with_padding() call when the dataset is already in device memory and no padding is needed. Previously, changing to kDevice would cause copy_with_padding() to allocate a second device copy of the full dataset, causing OOM on GPUs where the dataset already fills most of device memory.

  3. copy(): Reset sub_indices_ with placement new before invoking the copy constructor. When kDevice is used, sub_indices_ can be left in a corrupted state from a prior code path, causing UB in the copy constructor.

Notes

  • The fix applies to the ANN benchmark harness only; it does not change the CAGRA library API.
  • No existing unit tests cover filtered search through the benchmark harness memory-type path; this fix was validated via direct benchmark runs.

… filter bitset

cuvs_cagra::get_preference() returned MemoryType::kHostMmap, which the
benchmark framework used to provide the filter bitset as a file-backed
mmap pointer. GPU kernels cannot read host mmap memory on non-ATS GPUs
(A100, H100, V100), causing the filter to be silently non-functional and
recall to collapse to approximately the filter pass rate regardless of
itopk. On ATS-capable GPUs (GH200), the file-backed mmap pages are not
pre-faulted, triggering a cascade of concurrent ATS page faults during
graph traversal and producing incorrect results.

Fix 1: Change get_preference() to return MemoryType::kDevice so the
framework explicitly copies the filter bitset and dataset to device
memory before CAGRA's kernels access them. This is correct on all GPUs.

Fix 2: With kDevice, the framework copies the full dataset to device
before calling set_search_dataset(). The original set_search_param()
then called copy_with_padding() unconditionally, doubling device memory
usage (e.g. 2x 38.4 GB for deep-100M) and causing OOM. Skip
copy_with_padding() when the dataset is already on device and no
16-byte alignment padding is required.

Fix 3: sub_indices_ accumulates corrupted state when the kDevice path
triggers the dataset update code path. For single-index CAGRA,
sub_indices_ is always logically empty, so reset it via placement new
in copy() to prevent bad_array_new_length when the copy constructor runs.

Tested on H100 80GB (no ATS) and GH200 (ATS) with deep-1M/deep-100M,
5% filter pass rate, inner product distance. Recall with kHostMmap
(unpatched): 0.0495 across all itopk values. Recall with kDevice (fix):
0.45-0.9999 scaling correctly with itopk, reaching >0.99 at itopk=1024.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@copy-pr-bot

copy-pr-bot Bot commented Jul 7, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@jamxia155

Copy link
Copy Markdown
Contributor

The benchmark was using property.dataset_memory_type = MemoryType::kHostMmap which allowed datasets that cannot fit in GPU memory, as long as they can fit on host. We lose that capability with the current change to kDevice. An improvement would be to de-couple the memory type of the dataset from the filter, e.g. property.dataset_memory_type = MemoryType::kHostMmap and a new property.filter_memory_type = MemoryType::kDevice.

The PR description suggests that the issue was incorrect results without ATS or performance degradation with ATS. Am I remembering things right that you were using a GH200 system that was ATS-enabled, but the issue you ran into was with correctness?

Aside: for formatting, please run pre-commit run --files cpp/bench/ann/src/cuvs/cuvs_cagra_wrapper.h.

@cjnolet cjnolet added bug Something isn't working non-breaking Introduces a non-breaking change labels Jul 7, 2026
…emory_type

The original fix (kHostMmap -> kDevice for dataset_memory_type) corrected
the bug but removed support for datasets larger than GPU memory. This revision
restores kHostMmap for the dataset and introduces a separate filter_memory_type
property so the filter bitset can be placed on device independently.

Root cause: cuvs_cagra::get_preference() returned kHostMmap for both the
dataset and the filter bitset. CAGRA's search dispatch calls
bitset_view::count() on the filter, which wraps the bitset pointer in
make_device_vector_view and passes it to raft::popc. With a kHostMmap
pointer, this reads un-faulted file-backed mmap pages from the GPU:
- On ATS GPUs (GH200): GPU hardware reaches host memory, but un-faulted
  pages return zeros/garbage -> wrong filtering_rate -> incorrect recall
- On non-ATS GPUs (H100, A100): GPU cannot access host mmap at all ->
  garbage filter bits -> recall collapses to ~pass_rate (random results)

Fix: add filter_memory_type to algo_property (optional, defaults to
dataset_memory_type for backward compatibility). cuvs_cagra sets
filter_memory_type = kDevice so the benchmark framework explicitly copies
only the filter bitset to device memory, while the dataset remains in
kHostMmap to support large-dataset workloads.

Changes:
- ann_types.hpp: add std::optional<MemoryType> filter_memory_type to
  algo_property, defaulting to nullopt (backward compatible)
- benchmark.hpp: parse filter_memory_type from JSON config; use
  filter_memory_type.value_or(dataset_memory_type) when loading filter bitset
- cuvs_cagra_wrapper.h: set filter_memory_type = kDevice in get_preference()

Tested on H100 80GB (deep-1M, 5% filter, k=10). Unpatched: recall ~0.05
(random). Patched: recall 0.45-0.9999 scaling correctly with itopk.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working non-breaking Introduces a non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants